-
Couldn't load subscription status.
- Fork 32
feat(Anchor + Button): allow both leading and trailing icons #3141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
View your CI Pipeline Execution ↗ for commit 7e37ce1 ☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good! have a couple of comments around icon alignment + some story rendering
| export const InlineIcons: Story = { | ||
| render: () => ( | ||
| <FlexBox gap={16} row> | ||
| <FlexBox flexDirection={{ _: 'column', sm: 'row' }} gap={16}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something weird i've noticed is that disabled links no longer apply disabled styles and don't change over to buttons. this wasn't introduced in this PR but i'm curious when it started to happen
| iconPosition: 'right', | ||
| iconAndTextGap: 16, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: i'm sortof anti explicit style testing, but i think we can just think about cleaning these up when we eventually introduce visual testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alrighties, I forgot about this tbh and some of the tests were AI-gen tbh
I did remove the ones related to spacing
but I kept the layout related ones, thoughts?
📬Published Alpha Packages:@codecademy/[email protected] |
|
🚀 Styleguide deploy preview ready! |

Overview
Updates the Anchor and Button components to allow both leading and trailing icons
PR Checklist
Testing Instructions
Don't make me tap the sign.
?path=/docs/typography-anchor--docs&globals=viewport:responsiveand scroll down to the "Icons" section?path=/docs/atoms-buttons-button--docsand scroll to the "Inline icons" section/?path=/docs/molecules-menu--docsPR Links and Envs